Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add poetry #59

Merged
merged 5 commits into from
Feb 6, 2024
Merged

Add poetry #59

merged 5 commits into from
Feb 6, 2024

Conversation

ElliottKasoar
Copy link
Member

@ElliottKasoar ElliottKasoar commented Feb 5, 2024

Resolves #45 and #50

Updates pyproject.toml to be compatible with poetry, although I have omitted the .lock file to make this more general.

Notes:

  • poetry does not currently conform to PEP 621, so the format currently differs from most other modern pyproject.toml files, although support will be implemented in the next major release.
  • We cannot use dynamic versioning with this, so now need to specify the version both in pyproject.toml and init.py
  • Updates will be needed to replace the build/publishing, but these will be in a separate pull request

This also updates the workflows to use poetry installations where possible, bumps actions versions in workflows, and bumps dev (formerly testing) versions in pyproject.toml.

To do:

  • Update README with setup instructions

@ElliottKasoar ElliottKasoar marked this pull request as draft February 5, 2024 17:36
@ElliottKasoar ElliottKasoar marked this pull request as ready for review February 5, 2024 17:59
@alinelena alinelena self-requested a review February 5, 2024 20:41
@alinelena
Copy link
Member

  • Updates will be needed to replace the build/publishing, but these will be in a separate pull request

@ElliottKasoar can you open issues for these so we do not loose them once we merge the pr

@ElliottKasoar
Copy link
Member Author

  • Updates will be needed to replace the build/publishing, but these will be in a separate pull request

@ElliottKasoar can you open issues for these so we do not loose them once we merge the pr

I think this is already covered by #17 and #18

@ElliottKasoar
Copy link
Member Author

This will need rebasing once #58 is merged. I may also drop Python 3.12 back down to 3.11, as discussed with @alinelena, to minimise compatibility issues with janus(-core)

@alinelena
Copy link
Member

@ElliottKasoar i think some disconnect happened between the ci duplication removal pr #58 and this one... so practically your ci_orig changes need to be ported to ci.yaml... once done is mergeable

alinelena
alinelena previously approved these changes Feb 6, 2024
pyproject.toml Outdated Show resolved Hide resolved
alinelena
alinelena previously approved these changes Feb 6, 2024
oerc0122
oerc0122 previously approved these changes Feb 6, 2024
Copy link
Collaborator

@oerc0122 oerc0122 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine beyond that clarification for me. Though can you still install this e.g. through pip without poetry? Because for most people that's probably more convenient.

pyproject.toml Show resolved Hide resolved
@ElliottKasoar
Copy link
Member Author

Looks fine beyond that clarification for me. Though can you still install this e.g. through pip without poetry? Because for most people that's probably more convenient.

Assuming you mean pip installing the repository, pip install . will install the main dependencies (under [tool.poetry.dependencies]), but I don't think [docs] etc. works for the other grouped dependencies, which I suspect is the consequence of the current misalignment with other formats.

@alinelena alinelena merged commit ee0074e into stfc:main Feb 6, 2024
7 checks passed
@ElliottKasoar ElliottKasoar deleted the add-poetry branch February 6, 2024 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

check versions of deps
3 participants